move locks in TSRM.c to prevent races #8298
Conversation
move locks in TSRM.c to prevent races
|
Could you please provide more information on which race this is supposed to prevent? |
|
Hi, it seems the method that is not thread-safe should be protected by locks. I just move the locks to protect the missed variables. |
This could happen: flowchart TD
A[X Enters ts_allocate_id]
B[Y Enters ts_allocate_id]
A --> C[X Releases Mutex in error path]
B --> D[Y Acquires Mutex and travels success path]
C --> R[Race for rsrc_id]
D --> R
Although it's not surprising we don't see this in the real world ... In general the change makes sense to me, we otherwise only manipulate rsrc_id/offset inside critical sections, manipulating it outside does look wrong ... |
|
Since this appears to fix a potential bug, shouldn't this PR target PHP-8.0? |
Yes I did reply via email several days ago but the response seems to be missing. |
|
It's somewhere near the boundary that separates improvement and bugfix;
It's so unlikely that you would see this happen in the real world that I'm
comfortable saying it's impossible to write a test.
Still, it should probably target the lowest applicable branch, yes.
…On Tue, 5 Apr 2022 at 15:19, Christoph M. Becker ***@***.***> wrote:
Since this appears to fix a potential bug, shouldn't this PR target
PHP-8.0?
—
Reply to this email directly, view it on GitHub
<#8298 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AARB52RFNTRDCUUE3ENSEWTVDQ4VBANCNFSM5SNKYUYQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
move locks in TSRM.c to prevent races